-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement promote_to_multi
when converting WKB to sfc
#2369
base: main
Are you sure you want to change the base?
Conversation
promote_multi
when converting WKB to sfcpromote_to_multi
when converting WKB to sfc
There are still several tests failing for me on this PR:
Are these expected? |
I'll revisit in a bit! These are failing tests when specifically using the GDAL stream API, correct? If so, I think it is because nanoarrow implements "timezonelsss" timestamp conversion to R as assigning the UTC timestamp (for portability between the same code running on two computers...readr does this too). I may be remembering the details incorrectly! (I'm not sure about the CRS test case!) |
Apologies for taking so long to circle back here, but this is one approach to ensuring that
R CMD check
passes whenR_SF_ST_READ_USE_STREAM=true
. It also has the nice side-effect thatuse_stream = TRUE
no longer uses the wk package to construct sfc objects and that all users ofst_as_sfc(<WKB>)
can now usepromote_to_multi = TRUE
to get the same read-ogr-like behaviour.Closes #2296.
Example:
I do still see:
...when running
testthat::test_local()
withR_SF_ST_READ_USE_STREAM=true
. I think this happens because of how nanoarrow converts timestamps without an explicit timezone to R objects: nanoarrow assigns UTC as opposed to settingtzone = ""
or omitting it. I copied this behaviour from readr because it is more reproducible between systems, but I'm not sure it's any more or less correct.